-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[zk-sdk] Add the program
module
#1188
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1188 +/- ##
=========================================
- Coverage 82.1% 82.1% -0.1%
=========================================
Files 886 893 +7
Lines 236372 236414 +42
=========================================
+ Hits 194204 194212 +8
- Misses 42168 42202 +34 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's so nicely factored, I'm wondering if this should actually live in a separate crate under programs
. but we can split that out later
zk-sdk/src/program/instruction.rs
Outdated
//! [`ZK ElGamal proof`]: https://docs.solanalabs.com/runtime/zk-token-proof | ||
//! [`context-state`]: https://docs.solanalabs.com/runtime/zk-token-proof#context-data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we'll have to also fixup the docs, but that can happen later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, we'll definitely need to fix the docs!
zk-sdk/src/program/mod.rs
Outdated
pub mod state; | ||
|
||
// Program Id of the ZK ElGamal Proof program | ||
solana_program::declare_id!("ZkTokenProof1111111111111111111111111111111"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may as well just declare the new id, no? We can do ZkE1Gama1Proof11111111111111111111111111111
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
zk-sdk/src/errors.rs
Outdated
#[derive(Error, Clone, Debug, Eq, PartialEq)] | ||
pub enum ProofVerificationError { | ||
#[error("Invalid proof context")] | ||
ProofContext, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this rather be in the program
module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lHm yeah that is a good point. I will move it to the program module.
zk-sdk/src/program/mod.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason you decided on program
instead of elgamal-program
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reason. I thought we would just have a single program in the zk-sdk, but I guess it would make more sense to called it elgamal-program
. I'll make the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Problem
Each submodules from the
zk-token-sdk
are in the process of getting migrated tozk-sdk
. The submodules related to thezk-token-proof
program are yet to be moved over.Summary of Changes
Added the
program
module that contains logic related to the originalzk-token-proof
program.zk-token-proof
program is renamedzk-elgamal-proof
program. I kept the original id for the program for now. I think we will need to have a follow-up dedicated to changing this id.program
module instead of having it scattered throughout like in the zk-token-sdk.Fixes #